Skip to content

Conversation

@Akshay4754
Copy link

closes #252

java16DemoPool.add(new VectorAPIDemo());
// JEP 347
java16DemoPool.add(new Cpp14FeaturesDemo());
// Inside the list/map initialization in the JDK 16 helper file
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here only // JEP 375 comment should be placed, as for other demos

java16DemoPool.add(new Cpp14FeaturesDemo());
// Inside the list/map initialization in the JDK 16 helper file
// ... other demos ...
demos.add(new org.javademos.java16.jep357.MercurialToGitMigration()); // JEP 357
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with other demos, do not use absolute import path here. Use import at the begining of the file and here only class name.

import org.javademos.commons.IDemo;

/**
* ## JEP 357: Migrate from Mercurial to Git
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, change to Markdown format as in other demos. Check IDemo.java for reference.

public void demo() {
info(357);

System.out.println("This JEP was an infrastructure project to migrate the OpenJDK Community's source code repositories.");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The information are correct, but please, do not use println statements for explaining. The output would be too long if all demos be like that. Turn them to comments.

@AloisSeckar AloisSeckar added the changes requested The PR was checked and feedback provided. Changes must be done in order to get it accepted. label Oct 26, 2025
@AloisSeckar
Copy link
Owner

Since #313 was merged into main, which changes the way of loading the demo files, merge conflicts inevitably appeared in this PR and will have to be resolved. Please, sync your checkouts with current state of main. Or stash your changes, drop this PR and open a new one from a fresh code. I will help guiding you, if needed.

@AloisSeckar
Copy link
Owner

AloisSeckar commented Oct 30, 2025

@Akshay4754 will you be able to address the feedback and solve the merge conflicts? I tried to do it, but GitHub web editor cannot handle it, must be done locally in your checkout. Or you can create a new fork and re-create changes related to JEP 357.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes requested The PR was checked and feedback provided. Changes must be done in order to get it accepted.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JEP 357 - Migrate from Mercurial to Git

2 participants